-
Notifications
You must be signed in to change notification settings - Fork 176
Fix: Reset-AutomationSchedules incorrectly checking if provided time is at least one hour in the future #1834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Reset-AutomationSchedules incorrectly checking if provided time is at least one hour in the future #1834
Conversation
…er than one hour ago. Ensures UTC is compared to UTC instead of local.
|
Thank you for the contribution, @mpritchard2. Allow me some time to review and test the proposed change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Fixes a validation bug in Reset-AutomationSchedules.ps1 where the time validation logic was incorrectly checking if the provided time was in the past instead of ensuring it's at least one hour in the future.
Key changes:
- Corrected time comparison logic to validate future times instead of past times
- Fixed timezone handling by using DateTimeOffset instead of DateTime for consistent UTC comparisons
- Improved error message clarity to specify the requirement for future times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
|
@microsoft-github-policy-service agree company="Netchex" |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Approved - Bug fix correctly validates time is ≥1hr in future and uses DateTimeOffset for UTC comparison. Will add changelog post-merge since external fork.
|
@all-contributors please add @mpritchard2 for code |
|
@microsoft-github-policy-service[bot] I've updated the pull request to add @mpritchard2! 🎉 |
🛠️ Description
This fixes an issue in the validation of the provided time in Reset-AutomationSchedules.ps1. The current implementation was checking UTC - 1 hour against the local time of the parsed time string instead of UTC. The current implementation was also checking if sooner than an hour ago instead of sooner than an hour in the future.
Output of the original script from when I encountered the bug:
Please, enter a new base time for the weekly schedules in UTC (YYYY-MM-dd HH:mm:ss). If you want to keep the current one, just press ENTER: 2025-10-02 06:00:00
Exception: C:\Working\finops-toolkit\src\optimization-engine\Reset-AutomationSchedules.ps1:90
Line |
90 | throw "$newBaseTimeStr is an invalid base time. It can't be s …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| 2025-10-02 06:00:00Z is an invalid base time. It can't be sooner than 2025-10-02 02:56:01Z
📋 Checklist
🔬 How did you test this change?
🙋♀️ Do any of the following that apply?
📑 Did you update
docs/changelog.md?📖 Did you update documentation?